Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR removes a type assertion from the StackOneToolSet class that was forcing the result of defu(extraHeaders, baseHeaders) to be treated as the branded type StackOneHeaders. While removing unsafe type assertions is generally good practice, this specific change introduces a type compatibility issue.
Key Changes
- Removed
as StackOneHeaderstype assertion from the merged headers result in thecreateRpcToolmethod
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const extraHeaders = normalizeHeaders(additionalHeaders); | ||
| // defu merges extraHeaders into baseHeaders, both are already branded types | ||
| const actionHeaders = defu(extraHeaders, baseHeaders) as StackOneHeaders; | ||
| const actionHeaders = defu(extraHeaders, baseHeaders); |
There was a problem hiding this comment.
Removing the type assertion here will cause a type error. The defu function doesn't preserve TypeScript branded types, so the result will be inferred as Record<string, string> instead of StackOneHeaders. However, actionHeaders is passed to rpcAction() (lines 504, 512, 521) which expects StackOneHeaders | undefined according to the schema definition.
Instead of using a type assertion, consider validating the merged headers through the schema to properly maintain the branded type:
const actionHeaders = stackOneHeadersSchema.parse(defu(extraHeaders, baseHeaders));This ensures type safety while avoiding unsafe type assertions.
Summary by cubic
Removed the unnecessary "as StackOneHeaders" cast when merging headers into actionHeaders in StackOneToolSet. This fixes a TypeScript type error and lets the branded header type be inferred from baseHeaders and extraHeaders.
Written for commit d7a2a90. Summary will update on new commits.